MSC4446: Allow moving the fully read marker to older events#4446
MSC4446: Allow moving the fully read marker to older events#4446SpiritCroc wants to merge 4 commits into
Conversation
SpiritCroc
changed the title
tulir
added
proposal
There was a problem hiding this comment.
Implementation requirements:
- Server that ignores attempts to roll back without the flag
- Server that allows rolling back with the flag
- Client that uses the flag to intentionally roll back
m.fully_read - Testing to ensure old clients don't explode when
m.fully_readrolls back
There was a problem hiding this comment.
As mentioned in comments further down, I can currently offer:
- Server implementation via synapse PR (pending review): Support MSC4446: allow moving fully read markers backwards element-hq/synapse#19663
- Client implementation via SchildiChat Revenge / Rust SDK: SchildiChat/matrix-rust-sdk@e2d4ec2
Furthermore, here some clients that didn't explode on initial testing yet when rolling the marker back:
- SchildiChat Next (Element X Android base)
- SchildiChat Android Legacy (Element Android Classic base)
- SchildiChat Web (Element Web base, though on somewhat older 1.11.112 version)
- Beeper Android & Desktop (at the time of writing not implementing this MSC yet)
github-project-automation
Bot
moved this to Tracking for review
in Spec Core Team Workflow
|
Synapse PR / server implementation: element-hq/synapse#19663 |
|
SchildiChat Revenge implements this via SchildiChat/matrix-rust-sdk@e2d4ec2 / SchildiChat/ruma@54b2407 (client side code already allowed picking any message for setting the fully read marker, so only SDK-side usage of the flag was needed). In practice the client currently only invokes the |
anoadragon453
left a comment
anoadragon453
left a comment
There was a problem hiding this comment.
This looks fine overall to me just now. Currently providing a drive-by review as I review the Synapse PR.
Some small things below.
| follow similar rules as other endpoints which imply a certain message order, such as the | ||
| [`/messages`](https://spec.matrix.org/v1.18/client-server-api/#get_matrixclientv3roomsroomidmessages) endpoint. | ||
|
|
||
| If `allow_backward` is set to `true`, servers should also accept event IDs that move the fully read marker back in time. |
There was a problem hiding this comment.
And if allow_backward is true and the provided event ID matches the current state?
Personally, for idempotency's sake, I would recommend returning a 200 as there is no change to the state anyhow.
|
|
||
| Unstable implementations should use `com.beeper.allow_backward` in place of `allow_backward` in the request body. | ||
|
|
||
| Servers can promote support for this MSC in `/_matrix/client/versions` by setting the flag `com.beeper.msc4446`. |
There was a problem hiding this comment.
| Servers can promote support for this MSC in `/_matrix/client/versions` by setting the flag `com.beeper.msc4446`. | |
| Servers can promote support for this MSC in `/_matrix/client/versions` by setting the flag `com.beeper.msc4446` to `true`. |
| Unstable implementations should use `com.beeper.allow_backward` in place of `allow_backward` in the request body. | ||
|
|
||
| Servers can promote support for this MSC in `/_matrix/client/versions` by setting the flag `com.beeper.msc4446`. | ||
| The feature flag should continue to be advertised after the MSC is accepted until the server advertises support for the stable spec release that includes this MSC. |
There was a problem hiding this comment.
And presumably clients will have to use the unstable prefix in the request body unless they see the server advertising support for the stable spec release containing these changes?
4 participants
Rendered